Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade armon/go-metrics to hashicorp/go-metrics #30

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mkeeler
Copy link
Member

@mkeeler mkeeler commented Jun 9, 2023

This will require bumping to v0.5.0 to signify the breaking change.

This will require bumping to v0.5.0 to singify the breaking change.
@banks
Copy link
Member

banks commented Oct 20, 2023

@mkeeler is this waiting on anything? What makes the upgrade a breaking change?

@mkeeler
Copy link
Member Author

mkeeler commented Oct 20, 2023

Go will treat armon/go-metrics and hashicorp/go-metrics as two completely separate modules. Because of this the global metrics handlers they initialize are not shared. So what this means is that if your application pulls in armon/go-metrics and you update to use a library which pulls in hashicorp/go-metrics then all of your metrics will have disappeared. While nothing about it breaks the API, I see metrics emissions no longer functioning correctly without other updates to your application as something worthy of a version bump that signifies a breaking update.

As for if its waiting on anything I had originally drafted a slew of similar PRs:

memberlist: hashicorp/memberlist#287
serf: hashicorp/serf#693
raft-boltdb: hashicorp/raft-boltdb#33
raft: hashicorp/raft#558

I would still need to have updated even more libraries and exceeded the amount of time I could invest into updating the whole ecosystem of libraries that make their way into Consul (or nomad, vault, etc.)

IIRC I was running into some issues getting it all to work properly in Consul but I cannot remember exactly why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants